-
Notifications
You must be signed in to change notification settings - Fork 6
Refine Attribute Resolution #97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refine Attribute Resolution #97
Conversation
…uence vs dict typing.
|
Good timing, I'm talking to @koxudaxi about how to extract the rules into a spec for an LSP. If you feel confident these changes are final, I will jot them down. |
|
Not quite confident yet, I think we'd need to merge them and then try them out a bit and see if they are annoying to use. I'm not sure what format would be best overall since there are multiple things all intermingled together to get to the end result. |
|
@ianjosephwilson Looks like a policy I should get into the specification stuff I was doing for @koxudaxi ... if you can jot some thoughts down in a reply about styles or other attribute edge cases, I'll capture them. |
|
Apologies, clicked as my cursor travelled over the |
|
@pauleveritt Do you mean the rules for how to validate a t-string used by tdom ? |
|
@ianjosephwilson Sorry I though I mentioned in a discussion. @koxudaxi is reviving his PyCharm extension which teaches the IDE the tdom syntax. In particular, autocomplete and validation of props. As part of that, I did some stuff to decipher the tdom "spec" into a JSON file he could use as the rules. |
|
@davepeck Later this week when you get a chance I think this is a strong contender for merging but look it over. I'm going to put another note in the related issue. I just had it drafted because it was stacked on top of the other small PR you merged earlier. |
…ents or template functions.
|
@ianjosephwilson Thanks so much for this PR and for your patience while I, uh, put my feet up on a tropical beach for a couple weeks. :-) After reading this over in detail, I think it's a great stake in the ground and I'm happy to merge it and push an update to PyPi. My instinct is that we'll need to use it for a while to see how we feel about the behavior. I'm happy to make adjustments going forward, including large breaking changes (still pre-alpha!), as we learn more. Does this sound good to you? If you're feeling good about the PR, I'll go ahead and merge. PS: good riddance to |
|
Sorry I keep stacking commits on. This is a little bit more restrictive, as mentioned in #89 , but let's try this out and if we keep hitting cases where we need the other forms we can add them back in. A good overview is in |
|
@ianjosephwilson Those changes look good. TBH I'm somewhat skeptical that toggling will be easy for the typical dev to reason about in practice, but let's give it a go. Merging. |
Refine attribute resolution to allow for merging of
classandstyleattrs and smooth out a few edge cases.Summary
classvalues together internally as adict[str, bool]and output to astr()orNoneto omit when empty.stylevalues together internally as adict[str, str|None]and output to astr()orNoneto omit when empty.ariaanddataas before but do not omitNoneorFalseduring intermediary expansion.Major Changes
classchangesclass.str()values into a list of classes.['btn', ['btn-primary']]['btn', {'btn-primary': True}]Falsevalues fromdictform to allow classes previously merged to be "toggled" off.processor.py, dropclassnames()helper.stylechangesstyle.str()values into property name / value dictionary.Nonevalue fromdictform to allow styles previously merged to be "toggled" off.aria/datachangesNoneorFalseright away so that they may override prior values.Noneto allow fordef Comp(img_data=None)type pass-through optional argumentsTypes
Just for a quick understanding of how this could look these would be for the non-literal values: